Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Api/Auth #652

Merged
merged 18 commits into from
Jun 28, 2023
Merged

Api/Auth #652

merged 18 commits into from
Jun 28, 2023

Conversation

lowtorola
Copy link
Contributor

@lowtorola lowtorola commented Jun 20, 2023 •
edited
Loading

  • Generate Typescript types resource using drf-spectacular
  • Automate typegen procedure with .bat script
  • Generate ApiApi class with drf-spectacular
  • Write Api.ts class to replace Api.js from old frontend
  • Write Auth.ts class/auth function set for frontend authentication

@lowtorola lowtorola marked this pull request as draft June 20, 2023 21:21
@acrantel acrantel changed the base branch from main to frontend2 June 20, 2023 21:58
@lowtorola lowtorola marked this pull request as ready for review June 25, 2023 17:43
@lowtorola
Copy link
Contributor Author

I thiiiink this is ready for review? Hopefully there's no random stuff in there lol

acrantel
acrantel requested changes Jun 25, 2023
View reviewed changes
Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Left some comments and questions below.
one more ask: could you add rules to .prettierignore and .eslintrc.js to ignore the generated files?
afterwards, run npm run lint (or npm run format to automatically fix problems) and make sure that linting passes

frontend2/src/__test__/test_resources/battlecode_source_137.zip Outdated Show resolved Hide resolved
frontend2/src/__test__/utils.test.ts Outdated Show resolved Hide resolved
frontend2/src/__test__/utils.test.ts Outdated Show resolved Hide resolved
frontend2/src/utils/api.ts Outdated Show resolved Hide resolved
frontend2/src/utils/api.ts Show resolved Hide resolved
frontend2/src/utils/api.ts Outdated Show resolved Hide resolved
@acrantel
Copy link
Member

It looks like merging this PR will overwrite/undo the later changes I made in the routing pr because of the merge.
I think we can fix this by either deleting the files you merged in from my branch, or by undoing the merge (which might be hard).

@lowtorola
Copy link
Contributor Author

lowtorola commented Jun 26, 2023 •
edited
Loading

It looks like merging this PR will overwrite/undo the later changes I made in the routing pr because of the merge. I think we can fix this by either deleting the files you merged in from my branch, or by undoing the merge (which might be hard).

Oops! can I just merge frontend2 into my branch to fix this??

@lowtorola lowtorola requested a review from acrantel June 26, 2023 00:47
@acrantel acrantel changed the base branch from frontend2 to main June 27, 2023 03:38
@acrantel acrantel changed the base branch from main to frontend2 June 27, 2023 03:38
@acrantel
Copy link
Member

fixed the overwriting issues by changing the base branch around. looks good to merge now!

@lowtorola lowtorola merged commit 4b37303 into frontend2 Jun 28, 2023
2 checks passed
@lowtorola lowtorola deleted the frontend2-api_auth branch June 28, 2023 01:14
acrantel pushed a commit that referenced this pull request Aug 15, 2023
@lowtorola @acrantel
Api/Auth (#652)
3c47ff5 
Yayy API!!
acrantel pushed a commit that referenced this pull request Aug 15, 2023
@lowtorola @acrantel
Api/Auth (#652)
da992ec 
Yayy API!!
acrantel pushed a commit that referenced this pull request Oct 3, 2023
@lowtorola @acrantel
Api/Auth (#652)
3400e90 
Yayy API!!
acrantel pushed a commit that referenced this pull request Feb 8, 2024
@lowtorola @acrantel
Api/Auth (#652)
2a36c6e 
Yayy API!!
acrantel pushed a commit that referenced this pull request Feb 8, 2024
@lowtorola @acrantel
Api/Auth (#652)
5676d92 
Yayy API!!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acrantel acrantel acrantel approved these changes

@n8kim1 n8kim1 Awaiting requested review from n8kim1

@apollo1291 apollo1291 Awaiting requested review from apollo1291

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lowtorola @acrantel